Skip to content

Conversation

@trinistr
Copy link
Contributor

  • Include shared spec which has better descriptions and more tests.
  • Test Kernel#raise, not Kernel.raise (Kernel.raise has separate tests).
  • Remove duplicated tests.

There is more work to be done in organizing the tests:

  • Fiber and Thread seem to have some generic tests that are already included in the shared spec.
  • Tests for intricacies of re-raising exceptions could also be moved to the shared spec probably.

@trinistr trinistr force-pushed the improve-kernel-raise branch from 8d4fdf6 to 1e957b6 Compare January 19, 2026 11:33
@trinistr trinistr marked this pull request as ready for review January 19, 2026 11:33
cause = StandardError.new
-> { raise("error", cause: cause) }.should raise_error(RuntimeError) { |e| e.cause.should == cause }
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: haven't found a similar test in :kernel_raise_with_cause. Isn't it just being deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems I had code blindness when reviewing changes. I've added tests for this and the next one. Also added a context "without cause keyword argument", which now houses tests for what happens when cause is not specified.


it "doesn't raise a TypeError when given cause is nil" do
-> { raise "message", cause: nil }.should raise_error(RuntimeError, "message")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: similar issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment

raise
end
end.should raise_error(StandardError, "aaa")
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: the same issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Include shared spec which has better descriptions and more tests.
- Test Kernel#raise, not Kernel.raise (Kernel.raise has separate tests).
- Remove duplicated tests.

There is more work to be done in organizing the tests:
- Fiber and Thread seem to have some generic tests that are already included in the shared spec.
- Tests for intricacies of re-raising exceptions could also be moved to the shared spec probably.
@trinistr trinistr force-pushed the improve-kernel-raise branch from 1e957b6 to de25fdd Compare January 28, 2026 15:45
@trinistr
Copy link
Contributor Author

How did a test that doesn't exist on this branch fail on an Encoding that doesn't exist in Ruby? 😅

File.dirname rejects strings encoded with non ASCII-compatible encodings ERROR
Encoding::ConverterNotFoundError: code converter not found (UTF-8 to RS3UTF-16-BE)
/Users/runner/work/spec/spec/core/file/dirname_spec.rb:83:in `encode'

@eregon
Copy link
Member

eregon commented Jan 28, 2026

^ discussed in #1330 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants